Skip to content

[Transform][Fusion] yield fused producer if necessary #289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 29, 2024
Merged

[Transform][Fusion] yield fused producer if necessary #289

merged 2 commits into from
Aug 29, 2024

Conversation

@Yun-Fly
Copy link
Contributor

@Yun-Fly Yun-Fly commented Aug 27, 2024 •
edited
Loading

Automatically yield fused producer if it has multiple uses. E,g,

func.func @yield_fused_producer(%arg0: tensor<16x32x32xf32>) -> (tensor<16x32x32xf32>, tensor<16x32xf32>) {
    %cst_0 = arith.constant dense<2.000000e+00> : tensor<16x32x32xf32>
    %dest0 = tensor.empty() : tensor<16x32x32xf32>
    %0 = linalg.powf ins(%arg0, %cst_0 : tensor<16x32x32xf32>, tensor<16x32x32xf32>) outs(%dest0 : tensor<16x32x32xf32>) -> tensor<16x32x32xf32>
    %dest1 = tensor.empty() : tensor<16x32xf32>
    %1 = linalg.reduce { arith.addf } ins(%0 : tensor<16x32x32xf32>) outs(%dest1 : tensor<16x32xf32>) dimensions = [2]
    return %0, %1 : tensor<16x32x32xf32>, tensor<16x32xf32>
  }

When fusing powf into tiled consumer reduce, %0 should also be yield because it has two users. Otherwise, there would exist repeat powf computation.

@Yun-Fly Yun-Fly force-pushed the yunfei/yield_fused_producer branch from 7ab0442 to 3cfdafc Compare August 27, 2024 07:53
@Yun-Fly
Copy link
Contributor Author

Yun-Fly commented Aug 28, 2024

This patch is also helpful to debug intermediate result when correctness check failed.

@Yun-Fly Yun-Fly requested review from Menooker and ZhennanQin August 28, 2024 03:01
@Yun-Fly Yun-Fly force-pushed the yunfei/yield_fused_producer branch from 3cfdafc to 0f75ff9 Compare August 28, 2024 03:48
@Yun-Fly Yun-Fly force-pushed the yunfei/yield_fused_producer branch from 0f75ff9 to 6a52eba Compare August 28, 2024 05:25
@Yun-Fly Yun-Fly linked an issue Aug 28, 2024 that may be closed by this pull request
Eliminate redundant computation when fusing producer with multiple uses #298
Closed
@Yun-Fly Yun-Fly mentioned this pull request Aug 28, 2024
Closed
Menooker
Menooker reviewed Aug 28, 2024
View reviewed changes
Menooker
Menooker reviewed Aug 28, 2024
View reviewed changes
Menooker
Menooker reviewed Aug 28, 2024
View reviewed changes
lib/gc/Transforms/TilingUsingInterfaceX.cpp
void notifyOperationErased(Operation *op) override {
// Call old listener hook.
if (auto *oldListener =
dyn_cast_if_present<RewriterBase::Listener>(oldListenerHook))
Copy link

@Menooker Menooker Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this cast?

Copy link
Contributor Author

@Yun-Fly Yun-Fly Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notifyOperationErased is just the member function of RewriterBase::Listener, which is the sub-class of OpBuilder::Listener.

Copy link

@Menooker Menooker Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline discussed.

@ZhennanQin ZhennanQin merged commit b593976 into main Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yifeizh2 yifeizh2 Awaiting requested review from yifeizh2

@ZhennanQin ZhennanQin Awaiting requested review from ZhennanQin

2 more reviewers

@Menooker Menooker Menooker approved these changes

@WangJialei-A WangJialei-A WangJialei-A approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Eliminate redundant computation when fusing producer with multiple uses

5 participants

@Yun-Fly @Menooker @WangJialei-A @ZhennanQin